Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Don't link against kokkostools in tests unconditionally #252

Merged
merged 1 commit into from
May 30, 2024

Conversation

masterleinad
Copy link
Contributor

Tests would normally just use environment variables to load the respective tool. In any case, we shouldn't link to kokkostools unconditionally. Tests that set the respective hooks explicitly would not use the environment variable KOKKOS_TOOLS_LIBS and only link against kokkostools that can either happen explicitly or via another argument to kp_add_executable_and_test.

@vlkale
Copy link
Contributor

vlkale commented Apr 29, 2024

Thanks @masterleinad

Imposing kokkostools (monolithic) library requirement only for those Kokkos Tools tests needing it will make it easier for users to run the Kokkos Tools tests on their own platforms.
I think none of the tests in develop need more than the individual (not monolithic) library for KOKKOS_TOOLS_LIBS.

Copy link
Contributor

@vlkale vlkale left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

Maybe we put a comment here to developers of Kokkos Tools connectors to only use the monolithic library if it’s needed for a particular test (i.e., we explain the best practice that you conveyed in the description of the PR).

It would be equally fine with me to put this in internal-documents on Kokkos Tools for developers.

@crtrott crtrott merged commit 58258bd into kokkos:develop May 30, 2024
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants